Skip to content

fix(layer): ship the native addon versions dd-trace declares - #857

Merged
joeyzhao2018 merged 1 commit into
mainfrom
joey/layer-native-deps-from-dd-trace
Oct 29, 2026
Merged

joeyzhao2018 merged 1 commit into
mainfrom
joey/layer-native-deps-from-dd-trace

Conversation

@joeyzhao2018

@joeyzhao2018 joeyzhao2018 commented Oct 29, 2026 •

Copy link
Copy Markdown
Collaborator

What does this PR do?

Makes the layer ship @datadog/pprof and @datadog/native-appsec at the exact versions declared by the dd-trace it bundles, instead of at datadog-lambda-js's own devDependency pins.

  • scripts/move_ddtrace_dependency.js now takes the installed dd-trace package.json. It reads the dd-trace version and the two native addon versions (from dd-trace's optionalDependencies) from that file. The build fails with a clear error if dd-trace stops declaring either addon, instead of silently falling back to a stale pin.
  • Dockerfile passes node_modules/dd-trace/package.json to the script.
  • integration_tests_local/prepare-layer.js (and its README) does the same, so the local layer fixture keeps matching the release layer.

Motivation

profiling would be broken if we release a layer now.
Why it breaks?

  1. dd-trace starts sending "auto". dd-trace v5.128.0, v6.17.0 and master default DD_PROFILING_EXPERIMENTAL_OOM_HEAP_LIMIT_EXTENSION_SIZE to "auto" (dd-trace-js #10348) and pass it to pprof. Only pprof 5.19.0 and later understand "auto": their heap-profiler.js turns it into 0 plus a flag. dd-trace declares @datadog/pprof: 5.20.0, so its own code is correct.
  2. The layer build swaps in an older pprof. pprof is an optionalDependency of dd-trace. The datadog-lambda-js Dockerfile installs with yarn install --ignore-optional, then ships its own hand-maintained pin, "@datadog/pprof": "5.18.1". That pin matches the dd-trace 6.15.0 that main releases today. Your benchmark layer swapped in dd-trace master but kept the old pin.
  3. dd-trace turns the error into "no profiling at all". Profiler.start() catches the error, sets #startFailed, and stops every profiler permanently. It only logs this when DD_TRACE_DEBUG is on, which is why the info-level logs looked clean.

This pin has broken in both directions before. It started as "*", which drifted ahead of dd-trace, was then pinned to 5.14.4 (#783), and has now fallen behind dd-trace. Taking the versions from dd-trace removes the second source of truth.

Testing Guidelines

The script's output for each dd-trace manifest:

dd-trace @datadog/pprof @datadog/native-appsec Before this PR
6.15.0 (lockfile, Node 22+) 5.18.1 11.0.2 5.18.1 / * → 11.0.1
5.126.0 (Node 18/20 pin) 5.18.1 11.0.2 5.18.1 / * → 11.0.1
7.0.0-pre (dd-trace master) 5.20.0 11.0.3 5.18.1 / * → 11.0.1
  • Error paths: a missing dd-trace argument, and a dd-trace manifest that doesn't declare @datadog/pprof, both fail with a descriptive error.

  • prepare-layer.js against the installed tree writes dd-trace 6.15.0, pprof 5.18.1 and native-appsec 11.0.2 to deps.package.json.

  • The layer Docker build for every Node version runs in this PR's CI ("Build Layers for System Tests" and the GitLab layer build and size check).

  • End-to-end on Lambda nodejs26.x (serverless-e2e-tests test_profiling_usage, staging org): I built this branch's layer with dd-trace bumped to 6.22.0, which defaults to "auto" and declares pprof 5.20.0. The build pulled in pprof 5.20.0 and native-appsec 11.0.3. With no env overrides, across 22 invocations:

    • all three profilers started, with no uint32 errors;
    • the profile was accepted by intake.profile.datad0g.com;
    • datadog.profiling.agent reported, and test_profiling_usage[node26-profiling] passed.

    The same test against a layer bundling pprof 5.18.1 (built against dd-trace master) produced no profiles.

Additional Notes

  • Changes released layer contents at the next release: native-appsec 11.0.1 → 11.0.2, matching dd-trace 6.15.0. pprof is unchanged (5.18.1).
  • The @datadog/pprof and @datadog/native-appsec devDependencies stay as they are for local development and unit tests. They no longer decide what ships in the layer.
  • Layers built by other pipelines that bypass this Dockerfile (for example, ones that copy a dd-trace checkout over node_modules) aren't covered by this change.
  • A follow-up hardening idea for dd-trace (not needed for this fix): catch a failing OOM-monitor setup in NativeSpaceProfiler.start so it disables only OOM monitoring, not all profiling.

Types of Changes

  • Bug fix
  • New feature
  • Breaking change
  • Misc (docs, refactoring, dependency upgrade, etc.)

Check all that apply

  • This PR's description is comprehensive
  • This PR contains breaking changes that are documented in the description
  • This PR introduces new APIs or parameters that are documented and unlikely to change in the foreseeable future
  • This PR impacts documentation, and it has been updated (or a ticket has been logged)
  • This PR's changes are covered by the automated tests
  • This PR collects user input/sensitive content into Datadog
  • This PR passes the integration tests (ask a Datadog member to run the tests)

🤖 Generated with Claude Code

The layer is installed with --ignore-optional, which drops dd-trace's
optional native addons (@datadog/pprof, @datadog/native-appsec). The build
re-added them at our own devDependency pins, which drift from the versions
dd-trace calls into:

- dd-trace >= 5.128.0 / 6.17.0 defaults
  DD_PROFILING_EXPERIMENTAL_OOM_HEAP_LIMIT_EXTENSION_SIZE to "auto", which
  only @datadog/pprof >= 5.19.0 accepts. Against our 5.18.1 pin the
  profiler fails to start ("Heap limit extension size must be a uint32")
  and no profiles are sent. Released layers still ship dd-trace 6.15.0 and
  are not affected yet; a layer built against dd-trace master is.
- @datadog/native-appsec "*" resolves to 11.0.1 from the lockfile, while
  dd-trace 6.15.0 declares 11.0.2.

Take both from the installed dd-trace's optionalDependencies instead, and
do the same in the local integration test layer fixture.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@joeyzhao2018
joeyzhao2018 requested a review from a team as a code owner October 29, 2026 17:00
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 29, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-29T17:02:43.844461Z 4efdcfd PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@datadog-prod-us1-5 datadog-prod-us1-5 Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bits Code Review: PASS

More details

Targeted manifest and error-path checks support selecting native-addon versions from the bundled dd-trace manifest. Full layer builds remain unverified because dependencies were absent.

Was this helpful? React 👍 or 👎

Open Bits AI session

🤖 Bits Code Review · Commit 4efdcfd

@joeyzhao2018
joeyzhao2018 merged commit ba470c1 into main Oct 29, 2026
68 checks passed
@joeyzhao2018
joeyzhao2018 deleted the joey/layer-native-deps-from-dd-trace branch October 29, 2026 18:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants